Allow for discriminating between beta and stable when building releases on Buildkite #17893

mokagio · 2022-02-04T11:47:29Z

This builds on top of #17891 to address the beta vs stable issue that I didn't notice in previous code reviews. See #17891 (review).

The "Rationale" section below is quite verbose. The code should explain itself, but I thought of adding it for extra details. Before diving into it, I'll drop the testing details.

To Test

The difference between beta and stable is in generating GitHub releases, which can happen only if the ASC submission succeeds. But the ASC submission cannot succeed unless we change the build number, and we don't want to do that.

That's to say, the only way to be 100% sure this work is to run it "for real", after it's been merged.

I attempted to test it as far as it would go by hacking the build in a way to verify my changes behaved as planned. See details in #17892.

Rationale

Before explaining my approach, here's some context on how release builds work on Buildkite. It's actually pretty similar to how they worked on CircleCI, but worth stating here:

When a release manager runs a lane that should result in a new build, such as complete_code_freeze or finalize_release...
The lane internally calls either trigger_beta_build or trigger_release_build...
Which in turn call the new buildkite_trigger_build to run the pipeline described in release-builds.yml

The option I chose to differentiate between a beta release and a stable one was to call Buildkite with environment variable that is then converted to a call parameter for the command script, which in turn converts it into the appropriate beta_release:true|false for the build.

The other option I though of was to had two pipeline files for beta and stable, and have trigger_(beta|release)_build call the appropriate one. I decided against this option because it would have resulted in too much duplication. Besides it would have either still used the script parameter converted to beta_release argument option from the current implementation, or have even more duplication with a dedicated command script for the beta release.

You might also notice that I edited only the WordPress step, not the Jetpack one. That's because the only current effect of differentiating between beta and stable is in the resulting GitHub release being a pre-release or not. Since only the WordPress step generates the GitHub release, there was no need for the Jetpack one to change, too. Besides, the Jetpack build lane doesn't even expect a beta parameter:

WordPress-iOS/fastlane/Jetpack-Fastfile

Lines 90 to 118 in 9fc374b

    
           desc "Build for TestFlight" 
        
           lane :build_and_upload_jetpack_for_app_store do |options| 
        
             jetpack_appstore_code_signing 
        
             gym( 
        
               scheme: "Jetpack", 
        
               workspace: WORKSPACE_PATH, 
        
               clean: true, 
        
               export_team_id: get_required_env("EXT_EXPORT_TEAM_ID"), 
        
               output_directory: BUILD_PRODUCTS_PATH, 
        
               derived_data_path: DERIVED_DATA_PATH, 
        
               export_options: { method: "app-store" } 
        
             ) 
        
             testflight( 
        
               skip_waiting_for_build_processing: true, 
        
               team_id: "299112", 
        
               api_key_path: APP_STORE_CONNECT_KEY_PATH 
        
             ) 
        
             sentry_upload_dsym( 
        
               auth_token: get_required_env("SENTRY_AUTH_TOKEN"), 
        
               org_slug: 'a8c', 
        
               project_slug: 'jetpack-ios', 
        
               dsym_path: lane_context[SharedValues::DSYM_OUTPUT_PATH] 
        
             ) 
        
           end

Regression Notes

Potential unintended areas of impact
N.A.
What I did to test those areas of impact (or what existing automated tests I relied on)
N.A.
What automated tests I added (or what prevented me from doing so)
N.A.

I have completed the Regression Notes.
I have considered adding unit tests for my changes. N.A.
I have considered adding accessibility improvements for my changes. N.A.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. N.A.

This will also make the addition of a parameter to differentiate between beta and stable builds cleaner.

We'll add another one next, and the line length would have gotten too long

After adding a parameter to the `release-build-wordpress.sh` call, I got a permission denied error. See https://buildkite.com/automattic/wordpress-ios/builds/4907#226a7e18-7592-4e95-9064-2829b4cb7f12/381-383 I guess if the path to a script is the only value in the `command` node, then Buildkite calls it via `sh` (or maybe `$SHELL`), otherwise it runs it as an actual command within its shell, in which case if the script is not executable, it fails.

mokagio · 2022-02-04T11:50:19Z

.buildkite/commands/release-build-wordpress.sh

+bundle exec fastlane build_and_upload_app_store_connect \
+  skip_confirm:true \
+  create_gh_release:true \
+  beta_release:${1:-true} # use first call param, default to true for safety


The alternative could be to make $1 required and check for it at the start of the script.

This seemed more concise, and I didn't feel we needed refined input validation given these scripts are only meant to run via CI.

mokagio · 2022-02-04T11:51:09Z

fastlane/Fastfile

-  lane :build_and_upload_stable_release do |options|
-    build_and_upload_app_store_connect(options)
-  end
-


These above were just unused lanes from the CircleCI implementation that I noticed while looking around the file.

mokagio · 2022-02-04T11:53:07Z

fastlane/Fastfile

-      branch: options[:branch_to_build],
-      pipeline_file: 'release-builds.yml'
-    )
+    trigger_buildkite_release_build(branch: options[:branch_to_build], beta: false)


I also considered ditching the two lanes for a single one with a beta parameter. I decided to keep them because it can happen to have to start one such build from the terminal and in that case it's handy to have dedicated lanes and tab complete between the two.

mokagio · 2022-02-04T11:53:59Z

fastlane/Pluginfile

-# gem 'fastlane-plugin-wpmreleasetoolkit', git: 'git@github.com:wordpress-mobile/release-toolkit', branch: 'trunk'
-gem 'fastlane-plugin-wpmreleasetoolkit', '~> 2.3'
+gem 'fastlane-plugin-wpmreleasetoolkit', git: 'git@github.com:wordpress-mobile/release-toolkit', branch: 'trunk'
+# gem 'fastlane-plugin-wpmreleasetoolkit', '~> 2.3'


Before merging this (or #17891) we ought to ship a new version of the release toolkit.

v3.0.0 (wordpress-mobile/release-toolkit#334) is almost ready to go

Let's land this, then update #17891 to use the new version of the toolkit.

jkmassel · 2022-02-08T16:30:36Z

Thanks for handling this Gio!!

mokagio added 8 commits February 4, 2022 10:59

Point release toolkit to trunk to get buildkite_trigger_build action

3eeef7f

Remove build_and_upload_beta_release lane, recently become unused

ea7bdb7

Remove build_and_upload_stable_release lane, recently become unused

3c593f2

DRY calls to Buildkite action in Fastfile

7a94603

This will also make the addition of a parameter to differentiate between beta and stable builds cleaner.

Move each parameter in release-build-wordpress.sh to dedicate line

f8087cd

We'll add another one next, and the line length would have gotten too long

Allow release-build-wordpress.sh to specify whether to build a beta

ac614d5

Forward beta release parameter from Fastlane to Buildkite

a298114

mokagio changed the base branch from trunk to remove/circle-ci February 4, 2022 11:47

mokagio commented Feb 4, 2022

View reviewed changes

mokagio mentioned this pull request Feb 4, 2022

Remove CircleCI #17891

Merged

mokagio added the Tooling Build, Release, and Validation Tools label Feb 4, 2022

mokagio added this to the 19.2 milestone Feb 4, 2022

mokagio marked this pull request as ready for review February 4, 2022 12:59

mokagio requested a review from jkmassel February 4, 2022 12:59

mokagio mentioned this pull request Feb 4, 2022

Release 3.0.0 into trunk wordpress-mobile/release-toolkit#334

Merged

mokagio self-assigned this Feb 8, 2022

jkmassel approved these changes Feb 8, 2022

View reviewed changes

jkmassel merged commit 8db8fd3 into remove/circle-ci Feb 8, 2022

jkmassel deleted the remove/circle-ci-and-fix-beta-builds branch February 8, 2022 16:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow for discriminating between beta and stable when building releases on Buildkite #17893

Allow for discriminating between beta and stable when building releases on Buildkite #17893

Uh oh!

mokagio commented Feb 4, 2022 •

edited

Loading

Uh oh!

mokagio Feb 4, 2022

Uh oh!

mokagio Feb 4, 2022

Uh oh!

mokagio Feb 4, 2022

Uh oh!

mokagio Feb 4, 2022

Uh oh!

mokagio Feb 8, 2022

Uh oh!

jkmassel Feb 8, 2022

Uh oh!

mokagio Feb 8, 2022

Uh oh!

jkmassel commented Feb 8, 2022

Uh oh!

Uh oh!

	desc "Build for TestFlight"
	lane :build_and_upload_jetpack_for_app_store do \|options\|

	jetpack_appstore_code_signing

	gym(
	scheme: "Jetpack",
	workspace: WORKSPACE_PATH,
	clean: true,
	export_team_id: get_required_env("EXT_EXPORT_TEAM_ID"),
	output_directory: BUILD_PRODUCTS_PATH,
	derived_data_path: DERIVED_DATA_PATH,
	export_options: { method: "app-store" }
	)

	testflight(
	skip_waiting_for_build_processing: true,
	team_id: "299112",
	api_key_path: APP_STORE_CONNECT_KEY_PATH
	)

	sentry_upload_dsym(
	auth_token: get_required_env("SENTRY_AUTH_TOKEN"),
	org_slug: 'a8c',
	project_slug: 'jetpack-ios',
	dsym_path: lane_context[SharedValues::DSYM_OUTPUT_PATH]
	)

	end

Allow for discriminating between beta and stable when building releases on Buildkite #17893

Allow for discriminating between beta and stable when building releases on Buildkite #17893

Uh oh!

Conversation

mokagio commented Feb 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

To Test

Rationale

Regression Notes

Uh oh!

mokagio Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

mokagio Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

mokagio Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

mokagio Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

mokagio Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

jkmassel Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

mokagio Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

jkmassel commented Feb 8, 2022

Uh oh!

Uh oh!

mokagio commented Feb 4, 2022 •

edited

Loading